Conversation
WalkthroughReplaces legacy responsive header CSS with a fixed, full-width header container; introduces structured header classes (nav-left/center/right), component-level selectors and utility classes; expands granular media-query coverage and adjusts dropdown/mobile behaviors. Also updates one home stylesheet breakpoint. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/common/src/components/header.vue (1)
693-699:⚠️ Potential issue | 🟠 Major
height: autois functionally capped bymax-height: 290pxin the active state, clipping dropdown content.The
.dropdown-menusetsheight: autoat line 693, but withoverflow: hiddenandmax-height: 290pxin the.activestate (line 674), content exceeds this cap. The dropdown title, padding, and multiple items quickly exceed 290px, making items inaccessible to users.🔧 Suggested adjustment
& .active { .dropdown-menu { - max-height: 290px; + max-height: min(70vh, 560px); + overflow-y: auto; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/components/header.vue` around lines 693 - 699, The dropdown is being clipped by the hard cap in the active state; update the CSS for .dropdown-menu and its .active state so long content isn’t hidden: remove or override the fixed max-height: 290px on .dropdown-menu.active (or set it to none) and replace overflow: hidden with overflow-y: auto so the menu can expand/scroll as needed while preserving the transition on open; adjust selectors .dropdown-menu and .dropdown-menu.active accordingly.
🧹 Nitpick comments (1)
packages/common/src/components/header.vue (1)
981-985: Consider merging duplicatemax-width: 1150pxblocks.Line 981 adds another
@media screen and (max-width: 1150px)block while one already exists earlier. Consolidating them reduces override risk and makes breakpoint behavior easier to maintain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/components/header.vue` around lines 981 - 985, Duplicate `@media` screen and (max-width: 1150px) blocks exist; locate the existing `@media` screen and (max-width: 1150px) rule and move the new selector ".opentiny-design-header .nav-right .dropdown-menu .dropdown-content { gap: 0; }" into that existing block, remove the newly added separate block, and ensure no conflicting overrides remain (preserve selector specificity and ordering so the merged rule still applies as intended).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/common/src/components/header.vue`:
- Around line 914-953: The media query currently using "@media screen and
(max-width: 1720px)" is too broad and affects mobile; change it to target
tablet/desktop only by scoping to the 815px+ range (e.g. use a media query with
min-width: 815px and max-width: 1720px) so the .opentiny-design-header grid
block, and its child rules for .nav-left, .nav-center and .nav-right, do not
apply under the mobile breakpoint.
- Around line 955-967: The media query for .opentiny-design-header
(min-width:1721px && max-width:1854px) switches to display:grid but doesn't
declare grid-template-columns, which causes implicit tracks and unstable
placement of .nav-left, .nav-center and .nav-right; update that rule to define
explicit columns (e.g., grid-template-columns: auto 1fr auto) and, if needed,
assign explicit grid-column positions for .nav-left, .nav-center and .nav-right
to ensure left/center/right layout remains stable across that breakpoint.
---
Outside diff comments:
In `@packages/common/src/components/header.vue`:
- Around line 693-699: The dropdown is being clipped by the hard cap in the
active state; update the CSS for .dropdown-menu and its .active state so long
content isn’t hidden: remove or override the fixed max-height: 290px on
.dropdown-menu.active (or set it to none) and replace overflow: hidden with
overflow-y: auto so the menu can expand/scroll as needed while preserving the
transition on open; adjust selectors .dropdown-menu and .dropdown-menu.active
accordingly.
---
Nitpick comments:
In `@packages/common/src/components/header.vue`:
- Around line 981-985: Duplicate `@media` screen and (max-width: 1150px) blocks
exist; locate the existing `@media` screen and (max-width: 1150px) rule and move
the new selector ".opentiny-design-header .nav-right .dropdown-menu
.dropdown-content { gap: 0; }" into that existing block, remove the newly added
separate block, and ensure no conflicting overrides remain (preserve selector
specificity and ordering so the merged rule still applies as intended).
| @media screen and (max-width: 1720px) { | ||
| .opentiny-design-header { | ||
| display: grid; | ||
| grid-template-columns: 1fr auto; // 两列：左侧内容 + 右侧nav-right | ||
| grid-template-rows: auto auto; // 两行 | ||
| align-items: center; | ||
| .nav-left { | ||
| margin-top: 16px; | ||
| margin-right: 64px; | ||
| grid-column: 1; // 第1列 | ||
| grid-row: 1; // 第1行 | ||
| } | ||
| .nav-center { | ||
| grid-column: 1 / -1; // 跨越所有列 | ||
| grid-row: 2; // 第2行 | ||
| margin-left: 0 !important; // 移除原来的左边距 | ||
| flex-wrap: wrap; // 允许内部换行 | ||
| margin: 16px 0; // 上下间距10px | ||
| } | ||
| .nav-right { | ||
| margin-top: 16px; | ||
| grid-column: 2; // 第2列 | ||
| grid-row: 1; // 第1行 | ||
| margin-left: 0; // 移除auto margin | ||
| .top-menu { | ||
| .dropdown-menu { | ||
| .dropdown-content-title { | ||
| margin: 10px 0 0 112px; | ||
| } | ||
| .dropdown-100 { | ||
| margin-left: 96px; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file to see the full context around lines 914-953
head -n 960 packages/common/src/components/header.vue | tail -n 100Repository: opentiny/opentiny.design
Length of output: 2046
🏁 Script executed:
# Check the total line count to ensure we're looking at the right section
wc -l packages/common/src/components/header.vueRepository: opentiny/opentiny.design
Length of output: 114
🏁 Script executed:
# Search for media queries in the file to understand the breakpoint strategy
rg "@media" packages/common/src/components/header.vue -A 2 -B 1Repository: opentiny/opentiny.design
Length of output: 1371
Scope the 1720px grid rule to tablet/desktop only.
The media query @media screen and (max-width: 1720px) applies from 0px to 1720px, including the mobile range. This causes the grid layout, margins, and flex properties to conflict with the dedicated mobile layout (defined at @media screen and (max-width: 814px)). The margin-top values and grid positioning will unintentionally affect mobile views.
The codebase establishes a clear breakpoint convention using min-width: 815px for desktop/tablet. Update the query accordingly:
Suggested adjustment
-@media screen and (max-width: 1720px) {
+@media screen and (min-width: 815px) and (max-width: 1720px) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @media screen and (max-width: 1720px) { | |
| .opentiny-design-header { | |
| display: grid; | |
| grid-template-columns: 1fr auto; // 两列：左侧内容 + 右侧nav-right | |
| grid-template-rows: auto auto; // 两行 | |
| align-items: center; | |
| .nav-left { | |
| margin-top: 16px; | |
| margin-right: 64px; | |
| grid-column: 1; // 第1列 | |
| grid-row: 1; // 第1行 | |
| } | |
| .nav-center { | |
| grid-column: 1 / -1; // 跨越所有列 | |
| grid-row: 2; // 第2行 | |
| margin-left: 0 !important; // 移除原来的左边距 | |
| flex-wrap: wrap; // 允许内部换行 | |
| margin: 16px 0; // 上下间距10px | |
| } | |
| .nav-right { | |
| margin-top: 16px; | |
| grid-column: 2; // 第2列 | |
| grid-row: 1; // 第1行 | |
| margin-left: 0; // 移除auto margin | |
| .top-menu { | |
| .dropdown-menu { | |
| .dropdown-content-title { | |
| margin: 10px 0 0 112px; | |
| } | |
| .dropdown-100 { | |
| margin-left: 96px; | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| `@media` screen and (min-width: 815px) and (max-width: 1720px) { | |
| .opentiny-design-header { | |
| display: grid; | |
| grid-template-columns: 1fr auto; // 两列：左侧内容 + 右侧nav-right | |
| grid-template-rows: auto auto; // 两行 | |
| align-items: center; | |
| .nav-left { | |
| margin-top: 16px; | |
| margin-right: 64px; | |
| grid-column: 1; // 第1列 | |
| grid-row: 1; // 第1行 | |
| } | |
| .nav-center { | |
| grid-column: 1 / -1; // 跨越所有列 | |
| grid-row: 2; // 第2行 | |
| margin-left: 0 !important; // 移除原来的左边距 | |
| flex-wrap: wrap; // 允许内部换行 | |
| margin: 16px 0; // 上下间距10px | |
| } | |
| .nav-right { | |
| margin-top: 16px; | |
| grid-column: 2; // 第2列 | |
| grid-row: 1; // 第1行 | |
| margin-left: 0; // 移除auto margin | |
| .top-menu { | |
| .dropdown-menu { | |
| .dropdown-content-title { | |
| margin: 10px 0 0 112px; | |
| } | |
| .dropdown-100 { | |
| margin-left: 96px; | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/common/src/components/header.vue` around lines 914 - 953, The media
query currently using "@media screen and (max-width: 1720px)" is too broad and
affects mobile; change it to target tablet/desktop only by scoping to the 815px+
range (e.g. use a media query with min-width: 815px and max-width: 1720px) so
the .opentiny-design-header grid block, and its child rules for .nav-left,
.nav-center and .nav-right, do not apply under the mobile breakpoint.
| @media (min-width: 1721px) and (max-width: 1854px) { | ||
| .opentiny-design-header .nav-center { | ||
| margin-left: 70px; | ||
| grid-row: 1; | ||
| } | ||
| .opentiny-design-header { | ||
| display: grid; | ||
| align-items: center; | ||
| .nav-left, .nav-right { | ||
| grid-row: 1; // 第1行 | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the header.vue file
find . -name "header.vue" -type f | head -20Repository: opentiny/opentiny.design
Length of output: 111
🏁 Script executed:
# Check the content at the specified lines
if [ -f "packages/common/src/components/header.vue" ]; then
wc -l packages/common/src/components/header.vue
fiRepository: opentiny/opentiny.design
Length of output: 114
🏁 Script executed:
# Read the specific section mentioned in the review
sed -n '950,975p' packages/common/src/components/header.vueRepository: opentiny/opentiny.design
Length of output: 559
🏁 Script executed:
# Check all `@media` queries and grid definitions in the file
rg -n "display:\s*grid|grid-template" packages/common/src/components/header.vueRepository: opentiny/opentiny.design
Length of output: 236
🏁 Script executed:
# Look at the broader context around other breakpoints
sed -n '900,1050p' packages/common/src/components/header.vue | cat -nRepository: opentiny/opentiny.design
Length of output: 4107
🏁 Script executed:
# Check if there are any comments or TODOs about this breakpoint
rg -n "1721|1854|1720" packages/common/src/components/header.vueRepository: opentiny/opentiny.design
Length of output: 168
🏁 Script executed:
# Check if there are any JavaScript logic or Vue computed properties that might depend on grid layout
rg -A 5 -B 5 "header|nav-center|nav-left|nav-right" packages/common/src/components/header.vue | head -100Repository: opentiny/opentiny.design
Length of output: 3253
Define explicit grid columns in the 1721–1854 breakpoint.
At line 961, the container switches to grid but lacks grid-template-columns, unlike the adjacent max-width: 1720px breakpoint which explicitly defines grid-template-columns: 1fr auto;. Without explicit column definitions, the three-region layout (left/center/right) depends on implicit tracks and margin positioning, making it less maintainable and prone to layout shifts.
🔧 Suggested adjustment
`@media` (min-width: 1721px) and (max-width: 1854px) {
- .opentiny-design-header .nav-center {
- margin-left: 70px;
- grid-row: 1;
- }
.opentiny-design-header {
display: grid;
+ grid-template-columns: auto 1fr auto;
align-items: center;
- .nav-left, .nav-right {
- grid-row: 1; // 第1行
- }
+ .nav-left {
+ grid-column: 1;
+ grid-row: 1;
+ }
+ .nav-center {
+ grid-column: 2;
+ grid-row: 1;
+ margin-left: 70px;
+ }
+ .nav-right {
+ grid-column: 3;
+ grid-row: 1;
+ justify-self: end;
+ margin-left: 0;
+ }
}
} 📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @media (min-width: 1721px) and (max-width: 1854px) { | |
| .opentiny-design-header .nav-center { | |
| margin-left: 70px; | |
| grid-row: 1; | |
| } | |
| .opentiny-design-header { | |
| display: grid; | |
| align-items: center; | |
| .nav-left, .nav-right { | |
| grid-row: 1; // 第1行 | |
| } | |
| } | |
| } | |
| `@media` (min-width: 1721px) and (max-width: 1854px) { | |
| .opentiny-design-header { | |
| display: grid; | |
| grid-template-columns: auto 1fr auto; | |
| align-items: center; | |
| .nav-left { | |
| grid-column: 1; | |
| grid-row: 1; | |
| } | |
| .nav-center { | |
| grid-column: 2; | |
| grid-row: 1; | |
| margin-left: 70px; | |
| } | |
| .nav-right { | |
| grid-column: 3; | |
| grid-row: 1; | |
| justify-self: end; | |
| margin-left: 0; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/common/src/components/header.vue` around lines 955 - 967, The media
query for .opentiny-design-header (min-width:1721px && max-width:1854px)
switches to display:grid but doesn't declare grid-template-columns, which causes
implicit tracks and unstable placement of .nav-left, .nav-center and .nav-right;
update that rule to define explicit columns (e.g., grid-template-columns: auto
1fr auto) and, if needed, assign explicit grid-column positions for .nav-left,
.nav-center and .nav-right to ensure left/center/right layout remains stable
across that breakpoint.
|
@James-9696 麻烦贴下效果图哈~ |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/common/src/components/header.vue (2)
915-954:⚠️ Potential issue | 🟠 MajorScope this breakpoint to tablet/desktop only.
Line 915 uses
max-width: 1720pxwithout a lower bound, so it still applies in mobile widths and can conflict with the dedicated mobile layout rules. Restrict it to815px+.Suggested fix
-@media screen and (max-width: 1720px) { +@media screen and (min-width: 815px) and (max-width: 1720px) { .opentiny-design-header { display: grid; grid-template-columns: 1fr auto; grid-template-rows: auto auto; align-items: center;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/components/header.vue` around lines 915 - 954, The media query using max-width: 1720px is too broad and affects mobile; restrict it to apply only to tablet/desktop by changing the rule to a bounded range (e.g., min-width: 815px and max-width: 1720px) so the styles for .opentiny-design-header and its children (.nav-left, .nav-center, .nav-right and nested .top-menu/.dropdown-menu rules) do not override mobile-specific layouts; update the media query selector accordingly to include the min-width lower bound.
956-968:⚠️ Potential issue | 🟠 MajorDefine explicit grid columns in the 1721–1854 range.
At Line 962, grid layout is enabled but no
grid-template-columnsis declared, so placement relies on implicit tracks. Make left/center/right columns explicit to avoid layout drift.Suggested fix
`@media` (min-width: 1721px) and (max-width: 1854px) { - .opentiny-design-header .nav-center { - margin-left: 70px; - grid-row: 1; - } .opentiny-design-header { display: grid; + grid-template-columns: auto 1fr auto; align-items: center; - .nav-left, .nav-right { - grid-row: 1; // 第1行 + .nav-left { + grid-column: 1; + grid-row: 1; + } + .nav-center { + grid-column: 2; + grid-row: 1; + margin-left: 70px; + } + .nav-right { + grid-column: 3; + grid-row: 1; } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/components/header.vue` around lines 956 - 968, The media query enables grid on .opentiny-design-header but doesn't define columns, causing implicit tracks; inside the `@media` (min-width: 1721px) and (max-width: 1854px) rule add an explicit grid-template-columns declaration for .opentiny-design-header (e.g., three columns for left/center/right) and ensure .nav-left, .nav-center, .nav-right are placed into the intended columns (use grid-column on those selectors if needed) so layout doesn't rely on implicit tracks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/common/src/components/header.vue`:
- Around line 915-954: The media query using max-width: 1720px is too broad and
affects mobile; restrict it to apply only to tablet/desktop by changing the rule
to a bounded range (e.g., min-width: 815px and max-width: 1720px) so the styles
for .opentiny-design-header and its children (.nav-left, .nav-center, .nav-right
and nested .top-menu/.dropdown-menu rules) do not override mobile-specific
layouts; update the media query selector accordingly to include the min-width
lower bound.
- Around line 956-968: The media query enables grid on .opentiny-design-header
but doesn't define columns, causing implicit tracks; inside the `@media`
(min-width: 1721px) and (max-width: 1854px) rule add an explicit
grid-template-columns declaration for .opentiny-design-header (e.g., three
columns for left/center/right) and ensure .nav-left, .nav-center, .nav-right are
placed into the intended columns (use grid-column on those selectors if needed)
so layout doesn't rely on implicit tracks.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/common/src/components/header.vue (2)
957-969:⚠️ Potential issue | 🟠 MajorDefine explicit grid columns for the 1721-1854 layout.
This block switches to grid at Line 963 but does not define columns, making left/center/right placement rely on implicit tracks.
Suggested fix
`@media` (min-width: 1721px) and (max-width: 1854px) { - .opentiny-design-header .nav-center { - margin-left: 70px; - grid-row: 1; - } .opentiny-design-header { display: grid; + grid-template-columns: auto 1fr auto; align-items: center; - .nav-left, .nav-right { - grid-row: 1; // 第1行 - } + .nav-left { grid-column: 1; grid-row: 1; } + .nav-center { grid-column: 2; grid-row: 1; margin-left: 70px; } + .nav-right { grid-column: 3; grid-row: 1; justify-self: end; } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/components/header.vue` around lines 957 - 969, The media query switches .opentiny-design-header to display: grid but lacks explicit columns, so define a grid template (e.g., grid-template-columns: auto 1fr auto) inside the same block and assign explicit grid-column positions for .nav-left, .nav-center and .nav-right (e.g., grid-column: 1 / 2, 2 / 3, 3 / 4 respectively) so left/center/right placement is deterministic; update the rules that reference .opentiny-design-header, .nav-left, .nav-center, and .nav-right in that media query accordingly.
916-955:⚠️ Potential issue | 🟠 MajorConstrain this rule to desktop/tablet; it still leaks into mobile.
Line 916 currently targets
0-1720px, so it also applies under the mobile breakpoint and can override/compete with mobile layout behavior.Suggested fix
-@media screen and (max-width: 1720px) { +@media screen and (min-width: 815px) and (max-width: 1720px) { .opentiny-design-header { display: grid; grid-template-columns: 1fr auto; grid-template-rows: auto auto;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/components/header.vue` around lines 916 - 955, The rule currently uses only max-width: 1720px so it also applies on mobile; change the media query to restrict it to tablet/desktop by adding a min-width (e.g. use `@media` screen and (min-width: 768px) and (max-width: 1720px)) so .opentiny-design-header (and its children .nav-left, .nav-center, .nav-right, .top-menu .dropdown-menu, etc.) only apply on tablet/desktop and no longer leak into mobile styles.
🧹 Nitpick comments (1)
packages/common/src/components/header.vue (1)
971-1068: Consider collapsing the 10px-step breakpoint ladder into one fluid rule.The many micro-ranges are hard to maintain and brittle during future header tweaks. A single interpolated rule in this band is easier to evolve.
Refactor direction (single range)
-@media (min-width: 1740px) and (max-width: 1750px) { ... } -@media (min-width: 1751px) and (max-width: 1760px) { ... } -@media (min-width: 1761px) and (max-width: 1770px) { ... } -... -@media (min-width: 1852px) and (max-width: 1854px) { ... } +@media (min-width: 1740px) and (max-width: 1854px) { + .opentiny-design-header .nav-right .dropdown-menu .dropdown-content-title { + margin-left: clamp(290px, calc(290px + (100vw - 1740px) * 0.316), 326px); + } + .dropdown-100 { + margin-left: clamp(275px, calc(275px + (100vw - 1740px) * 0.307), 310px); + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/components/header.vue` around lines 971 - 1068, The CSS contains many 10px-step media queries adjusting margins for .opentiny-design-header .nav-right .dropdown-menu .dropdown-content-title and .dropdown-100 which is brittle; replace the ladder with a single media query covering the whole range (the combined min/max currently spanned) and compute margins fluidly (using calc() or clamp() with vw/em offsets) instead of per-10px rules so the title and .dropdown-100 margins scale smoothly; update the selectors .opentiny-design-header .nav-right .dropdown-menu .dropdown-content-title and .dropdown-100 inside that single media query and remove the individual micro-range blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/common/src/components/header.vue`:
- Around line 957-969: The media query switches .opentiny-design-header to
display: grid but lacks explicit columns, so define a grid template (e.g.,
grid-template-columns: auto 1fr auto) inside the same block and assign explicit
grid-column positions for .nav-left, .nav-center and .nav-right (e.g.,
grid-column: 1 / 2, 2 / 3, 3 / 4 respectively) so left/center/right placement is
deterministic; update the rules that reference .opentiny-design-header,
.nav-left, .nav-center, and .nav-right in that media query accordingly.
- Around line 916-955: The rule currently uses only max-width: 1720px so it also
applies on mobile; change the media query to restrict it to tablet/desktop by
adding a min-width (e.g. use `@media` screen and (min-width: 768px) and
(max-width: 1720px)) so .opentiny-design-header (and its children .nav-left,
.nav-center, .nav-right, .top-menu .dropdown-menu, etc.) only apply on
tablet/desktop and no longer leak into mobile styles.
---
Nitpick comments:
In `@packages/common/src/components/header.vue`:
- Around line 971-1068: The CSS contains many 10px-step media queries adjusting
margins for .opentiny-design-header .nav-right .dropdown-menu
.dropdown-content-title and .dropdown-100 which is brittle; replace the ladder
with a single media query covering the whole range (the combined min/max
currently spanned) and compute margins fluidly (using calc() or clamp() with
vw/em offsets) instead of per-10px rules so the title and .dropdown-100 margins
scale smoothly; update the selectors .opentiny-design-header .nav-right
.dropdown-menu .dropdown-content-title and .dropdown-100 inside that single
media query and remove the individual micro-range blocks.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/common/src/components/header.vue (2)
517-520: Consider using CSS custom properties for the dropdown margin.The
margin-left: 269pxis a magic number that gets overridden by 15+ media queries later in the file. Using a CSS custom property would make maintenance easier and clarify the relationship between breakpoints.♻️ Suggested approach
.dropdown-100 { width: 100%; - margin-left: 269px; + margin-left: var(--dropdown-offset, 269px); }Then override
--dropdown-offsetin each media query instead of the full property.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/components/header.vue` around lines 517 - 520, The .dropdown-100 rule uses a hardcoded margin-left: 269px which is a magic number and is repeatedly overridden in many media queries; replace the hardcoded value with a CSS custom property (e.g. use margin-left: var(--dropdown-offset, 269px)) in the .dropdown-100 selector and then update each media query to set --dropdown-offset to the breakpoint-specific value so only the custom property is overridden instead of duplicating the full margin-left rule.
971-1068: Consider refactoring the granular 10px-increment media queries.There are 12 separate media queries for narrow 10-15px ranges (1740-1854px), each adjusting margins by small increments. This pattern is fragile and difficult to maintain—any change to the header layout will require updating all these breakpoints.
This suggests the dropdown positioning should use a more flexible approach:
♻️ Alternative approaches
- Use
calc()with viewport units for proportional positioning:.dropdown-content-title { margin-left: calc(15vw + 20px); }
Use CSS Grid or Flexbox to let the dropdown content align naturally with the header regions rather than absolute pixel offsets.
Use a single clamp() for the margin range:
.dropdown-content-title { margin-left: clamp(270px, 16vw, 330px); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/components/header.vue` around lines 971 - 1068, The repeated 10–15px-range media queries for .opentiny-design-header .nav-right .dropdown-menu .dropdown-content-title and .dropdown-100 are brittle; replace them with a single responsive rule (e.g., use clamp()/calc() with viewport units or switch to flexbox/grid alignment) that expresses margin-left as a fluid value across the whole range, remove all those specific `@media` blocks, and update .dropdown-100 to derive its offset from the same responsive formula or from container alignment so both elements stay in sync across viewport widths; verify visually around 1740–1854px after change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/common/src/components/header.vue`:
- Around line 517-520: The .dropdown-100 rule uses a hardcoded margin-left:
269px which is a magic number and is repeatedly overridden in many media
queries; replace the hardcoded value with a CSS custom property (e.g. use
margin-left: var(--dropdown-offset, 269px)) in the .dropdown-100 selector and
then update each media query to set --dropdown-offset to the breakpoint-specific
value so only the custom property is overridden instead of duplicating the full
margin-left rule.
- Around line 971-1068: The repeated 10–15px-range media queries for
.opentiny-design-header .nav-right .dropdown-menu .dropdown-content-title and
.dropdown-100 are brittle; replace them with a single responsive rule (e.g., use
clamp()/calc() with viewport units or switch to flexbox/grid alignment) that
expresses margin-left as a fluid value across the whole range, remove all those
specific `@media` blocks, and update .dropdown-100 to derive its offset from the
same responsive formula or from container alignment so both elements stay in
sync across viewport widths; verify visually around 1740–1854px after change.
2 participants
1、
2、
3、
4、
Summary by CodeRabbit
Summary by CodeRabbit